Skip to content

fast-import: fix lookup for new commits - #2252

Open
becm wants to merge 1 commit into
gitgitgadget:masterfrom
becm:fix-fast-import-commit-from-existing
Open

becm wants to merge 1 commit into
gitgitgadget:masterfrom
becm:fix-fast-import-commit-from-existing

Conversation

@becm

@becm becm commented Oct 9, 2026 •

Copy link
Copy Markdown

cc: Aniket Gupta aniketgupta3001@gmail.com

@gitgitgadget

gitgitgadget Bot commented Oct 9, 2026

Copy link
Copy Markdown

There is an issue in commit f4b298b:
fast-import: fix lookup for new commits

  • Commit not signed off

@becm
becm force-pushed the fix-fast-import-commit-from-existing branch 2 times, most recently from 2def667 to 90729d9 Compare October 9, 2026 23:29
Queries for commits created during the same 'git fast-import'  session
result in a crash:
    < reset refs/heads/newbranch
    < from <new-commit-oid>
    > fatal: not a valid commit: <new-commit-oid>

Getting file content for commit has no issue (working implementation):
    < ls <new-commit-oid> file-in-new-commit.txt
    > {file content}

During import, new objects may not (yet) be reachable via the ODB layer.
Try to resolve requested tag/commit via object cache first
to avoid crash and (eventually) improve lookup speed.

Signed-off-by: Marc Becker <becm@gmx.de>
@becm
becm force-pushed the fix-fast-import-commit-from-existing branch from 90729d9 to 4e50f63 Compare October 10, 2026 14:54
@becm

becm commented Oct 10, 2026

Copy link
Copy Markdown
Author

/submit

@gitgitgadget

gitgitgadget Bot commented Oct 10, 2026

Copy link
Copy Markdown

Submitted as pull.2252.git.1791653078449.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2252/becm/fix-fast-import-commit-from-existing-v1

To fetch this version to local tag pr-2252/becm/fix-fast-import-commit-from-existing-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2252/becm/fix-fast-import-commit-from-existing-v1

@gitgitgadget

gitgitgadget Bot commented Oct 11, 2026

Copy link
Copy Markdown

Aniket Gupta wrote on the Git mailing list (how to reply to this email):

Hello Marc,

Sorry for the noise, my first mail lost its In-Reply-To header on the
way, so it is not in this thread. Resending the same review here.

I tried this on top of 6de20f609 with DEVELOPER=1 and did not get warnings.

> Queries for commits created during the same 'git fast-import'  session
> result in a crash:
>     < reset refs/heads/newbranch
>     < from <new-commit-oid>
>     > fatal: not a valid commit: <new-commit-oid>

I was able to repro this. With the patch fsck is clean and it works
nicely. Same goes for "commit" with "from <new-oid>", a tag made in
the same session, and a tag of a tag. Existing commits and tags and
"from <blob>" behave like before.

One small nitpick: there is a double space in "'git fast-import'  session".

> to avoid crash and (eventually) improve lookup speed.

Did you measure this? If not, maybe drop it or say it is only a
possible side effect.

> +static struct object_entry *dereference(struct object_entry *oe, struct object_id *oid);

This is over 80 columns, can it be wrapped like the definition?

>  builtin/fast-import.c | 18 ++++++++++++++----

I could not find a test for this. Can you add one to t9300? You can
get the new oid with get-mark and use it in a "reset ... from <oid>"
in the same stream.

Thanks,
Aniket

@gitgitgadget

gitgitgadget Bot commented Oct 11, 2026

Copy link
Copy Markdown

User Aniket Gupta <aniketgupta3001@gmail.com> has been added to the cc: list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant